Skip to content

chore: Update proc_installing-che-on-openshift-with-keycloak-as-oidc.…#3112

Open
tolusha wants to merge 2 commits into
mainfrom
updateinstallingcheonopenshift
Open

chore: Update proc_installing-che-on-openshift-with-keycloak-as-oidc.…#3112
tolusha wants to merge 2 commits into
mainfrom
updateinstallingcheonopenshift

Conversation

@tolusha

@tolusha tolusha commented May 28, 2026

Copy link
Copy Markdown
Contributor

…adoc

What does this pull request change?

Updates proc_installing-che-on-openshift-with-keycloak-as-oidc

What issues does this pull request fix or reference?

eclipse-che/che#23831

Specify the version of the product this pull request applies to

main

Pull Request checklist

The author and the reviewers validate the content of this pull request with the following checklist, in addition to the automated tests.

  • Any procedure:
    • Successfully tested.
  • Any page or link rename:
    • The page contains a redirection for the previous URL.
    • Propagate the URL change in:
  • Builds on Eclipse Che hosted by Red Hat.
  • the Validate language on files added or modified step reports no vale warnings.

…adoc

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@tolusha tolusha requested a review from svor May 28, 2026 12:07
@github-actions

github-actions Bot commented May 28, 2026

Copy link
Copy Markdown

Click here to review and test in web IDE: Contribute

@tolusha

tolusha commented May 28, 2026

Copy link
Copy Markdown
Contributor Author

@gtrivedi88
Could you review pls

@tolusha

tolusha commented Jun 1, 2026

Copy link
Copy Markdown
Contributor Author

@gtrivedi88
Could you review pls

@gtrivedi88 gtrivedi88 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the delay in review. I have approved the PR with some suggestions. Also, for the sticky comment to resolve, probably we need to rebase with main. I fixed it last month.

... Click *Next*.
.. On the *Login settings* page:
+
... Entry `{prod-short}` redirect URL in the *Valid redirect URIs* field.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: "Entry" should be "Enter"

American English grammar issue - should be the imperative verb "Enter" not the noun "Entry".

Fix: "Enter {prod-short} redirect URL in the Valid redirect URIs field."

... Click *Next*.
.. On the *Login settings* page:
+
... Entry `{prod-short}` redirect URL in the *Valid redirect URIs* field.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JTBD Logic Flow Issue: User needs redirect URL before this step

This step asks users to enter the redirect URL but doesn't explain how to get it until the NOTE block several lines later. This breaks the user journey flow.

Suggested fix: Move the NOTE block (lines 44-54) to appear before step 1, or add a reference here like:
"Enter {prod-short} redirect URL (see NOTE below for command to obtain this URL) in the Valid redirect URIs field."

This follows JTBD principle of providing users with information when they need it.

----

. Create a secret for the OAuth client in the {prod-short} namespace:
. Create a Secret:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CQA 2.1 Issue: Step title too vague

"Create a Secret" doesn't tell users what the secret is for. CQA 2.1 requires clear, descriptive step titles that help users understand the purpose.

Suggested: "Create a Secret for OAuth authentication"

This makes the purpose clear and follows CQA 2.1 guidance for user-centric step descriptions.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CQA 2.1 Violation: Abstract format issue

The abstract should use WHY-first format per CQA 2.1 standards.

Current: "To manage user authentication through a centralized identity provider, deploy {prod-short}..."

Fix: "To enable centralized user authentication through an external identity provider, install {prod-short} on {orch-name} with {keycloak} as the OIDC provider."

Changes needed:

  • WHY (enable centralized auth) should come first
  • "deploy" → "install" to match procedure title

Signed-off-by: Anatolii Bazko <abazko@redhat.com>
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown

🎊 Navigate the preview: https://6a28165669f55b169eb877df--eclipse-che-docs-pr.netlify.app 🎊

@tolusha

tolusha commented Jun 9, 2026

Copy link
Copy Markdown
Contributor Author

Remarks have been addressed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants